Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve goto_file_impl #9065

Merged
merged 23 commits into from
Apr 6, 2024
Merged

Improve goto_file_impl #9065

merged 23 commits into from
Apr 6, 2024

Conversation

TornaxO7
Copy link
Contributor

continues #9038

Closes #8822 #8824

@TornaxO7 TornaxO7 marked this pull request as draft December 12, 2023 21:32
@TornaxO7
Copy link
Contributor Author

@ModProg

may I ask if you can try it out?

@TornaxO7 TornaxO7 changed the title adding shellexpand and extend surround_chars improve goto_file_impl Dec 12, 2023
@TornaxO7 TornaxO7 marked this pull request as ready for review December 13, 2023 22:02
@TornaxO7
Copy link
Contributor Author

TornaxO7 commented Dec 13, 2023 •
edited
Loading

File path detection strategy

I decided to read the chars next to the cursor one by one because:

  1. using treesitter requires a parser which doesn't exist for text or log files
    although paths could occur there
  2. paths are short enough to be read char by char in my opinion without waiting too
    long

Other information

So I tested it on $HOME/test.txt and ~/test.txt. Both opened up ~/test.txt as expected
but I don't know how to write tests for them.

@TornaxO7 TornaxO7 mentioned this pull request Dec 13, 2023
Closed
@ModProg
Copy link

ModProg commented Dec 13, 2023

I'd consider adding , to the characters terminating a path (like ;).

@TornaxO7
Copy link
Contributor Author

I'd consider adding , to the characters terminating a path (like ;).

added

TornaxO7
TornaxO7 commented Dec 13, 2023
View reviewed changes
helix-term/src/commands.rs Outdated
Comment on lines 1183 to 1190
&[
'@', '/', '\\', '.', '-', '_', '+', '#', '$', '%', '{', '}', '[', ']', ':',
'!', '~', '=',
]
} else {
&['@', '/', '.', '-', '_', '+', '#', '$', '%', '~', '=']
};
valid_chars.contains(c) || c.is_alphabetic()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked that up from neovim. See :h isf in neovim for the list of chars.

@kirawi kirawi added C-enhancement Category: Improvements A-command Area: Commands labels Dec 22, 2023
pascalkuthe
pascalkuthe requested changes Jan 8, 2024
View reviewed changes
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@pascalkuthe pascalkuthe added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2024
@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 10, 2024
the-mikedavis
the-mikedavis requested changes Feb 10, 2024
View reviewed changes
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis removed the S-waiting-on-review Status: Awaiting review from a maintainer. label Feb 10, 2024
@TornaxO7
Copy link
Contributor Author

I can't execute the integration tests. Executing cargo integration-test gives me:

error[E0412]: cannot find type `Id` in module `tokio::runtime`
  --> helix-event/src/runtime.rs:45:64
   |
45 |         parking_lot::RwLock<hashbrown::HashMap<tokio::runtime::Id, &'static T, ahash::RandomState>>,
   |                                                                ^^ not found in `tokio::runtime`

   Compiling gix-tempfile v13.0.0
error[E0599]: no method named `id` found for struct `Handle` in the current scope
  --> helix-event/src/runtime.rs:67:52
   |
67 |         let id = tokio::runtime::Handle::current().id();
   |                                                    ^^ method not found in `Handle`

Some errors have detailed explanations: E0412, E0599.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `helix-event` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 24, 2024
@the-mikedavis the-mikedavis mentioned this pull request Apr 6, 2024
Closed
@the-mikedavis the-mikedavis mentioned this pull request Apr 6, 2024
Closed
the-mikedavis
the-mikedavis approved these changes Apr 6, 2024
View reviewed changes
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

helix-term/src/commands.rs
.count();

let path: Cow<str> = text
.slice((start_pos - prefix_len)..(start_pos + postfix_len))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For readers' context: rope slicing is by character index rather than byte index (like &str) which is why this uses chars_at(..).count() rather than a len of a substring

@the-mikedavis the-mikedavis changed the title improve goto_file_impl Improve goto_file_impl Apr 6, 2024
@the-mikedavis the-mikedavis merged commit e69292e into helix-editor:master Apr 6, 2024
6 checks passed
@pascalkuthe pascalkuthe mentioned this pull request Apr 14, 2024
Closed
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 2024
@TornaxO7 @postsolar
Improve goto_file_impl (helix-editor#9065)
54cf0ae
@kirawi kirawi mentioned this pull request May 11, 2024
Closed
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
@TornaxO7 @mtoohey31
Improve goto_file_impl (helix-editor#9065)
8676130
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
@TornaxO7 @Vulpesx
Improve goto_file_impl (helix-editor#9065)
f2963f5
@krish-r krish-r mentioned this pull request Jun 14, 2024
Closed
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
@TornaxO7
Improve goto_file_impl (helix-editor#9065)
e9dd78e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalkuthe pascalkuthe pascalkuthe approved these changes

@the-mikedavis the-mikedavis the-mikedavis approved these changes

Assignees
No one assigned
Labels
A-command Area: Commands C-enhancement Category: Improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

goto_file path with ~
5 participants
@TornaxO7 @ModProg @the-mikedavis @pascalkuthe @kirawi